engineer: add EnforceFormRequestToDtoRule (queue #55 instance 2, Phase-2 promotion)#33
Conversation
…e-2 promotion) Flags concrete FormRequest subclasses that neither declare nor inherit a toDto() method. Doctrine: ADR-0012 (FormRequest -> DTO Flow). Identifier: enforceFormRequestToDto.missingToDtoMethod. Promoted from entreezuil's reflection-based Pest arch test (tests/Arch/FormRequestsTest.php) — the second instance of the 'arch test detects misuse but not omission' shape under war-room enforcement queue #55 (WR-0066). Sister of EnforceResourceDataValidatorOptInRule (instance 3, PR #20): same parameterized-base shape via the formRequestBaseClass PHPStan parameter (default Illuminate\Foundation\Http\FormRequest). Abstract classes exempt (BaseFormRequest intermediates); inherited and trait-provided toDto() satisfy the contract (method_exists parity with the source-of-truth Pest matcher); short-name collisions do not match (FQCN ancestor traversal). Per-territory legitimate exemptions (LoginRequest) migrate to consumer phpstan.neon ignoreErrors per package convention. Implementation note: constructor default uses FormRequest::class instead of an FQCN string literal — Pint's class_keyword fixer class_exists()'s class-shaped string literals and the Pint phar bundles a real FormRequest with an unbundled trait dependency, fataling composer format. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CHANGELOG [Unreleased] Added entry with candidate-Major versioning flag and pre-cascade audit demand; CLAUDE.md rules-shipped table row + Phase 2 narrative (third Phase 2 addition); README rules table row + configurable base class and exemptions section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Goosterhof
added
the
Agent Review Requested
Goosterhof
left a comment
Goosterhof
left a comment
There was a problem hiding this comment.
⚠️ Concerns
0 blockers · 2 concerns · 1 nit · 1 praise · 3 inline
Adds EnforceFormRequestToDtoRule (queue #55 instance 2) on the PR #20 parameterized-base template — rule logic is sound, the FQCN-traversal and abstract-exemption shapes are fixture-proven, CI is green on both PHP lanes, and the PR body matches the diff. Two test-coverage/API-currency concerns keep this short of approve-worthy; neither blocks.
Findings (detail inline)
⚠️ tests/Rules/EnforceFormRequestToDtoRuleTest.php:49— trait-providedtoDto()and the violating-transitive path are claimed but untested⚠️ src/Rules/EnforceFormRequestToDtoRule.php:120—ClassReflection::isSubclassOf(string)is deprecated in the vendored PHPStan 2.2.2- nit:
src/Rules/EnforceFormRequestToDtoRule.php:102 - praise: the
FormRequest::classconstructor default (src/Rules/EnforceFormRequestToDtoRule.php:178) dodging the Pint-pharclass_exists()fatal is a genuinely non-obvious correctness move — and backing it with a minimal reproduction plus a docblock explaining why theuseimport is alias-only is exactly how a landmine should be documented.
Out-of-scope declarations (routes-->can() sister WR-0067, issue #14 ratchet WR-0064, release PR) are acknowledged and stay out of scope.
Automated war-room agent review — posted because this PR carries the Agent Review Requested label.
Review Loop · 3/10 · FAIL — 🛑 1 🔴 1 🟡 1phpstan-warroom-rules #33 · AC anchor: none Caution 3 finding(s) posted inline — see the review comments on the changed lines. A BLOCKER must be resolved before merge. Actionescalate — BLOCKERs present |
1 similar comment
Review Loop · 3/10 · FAIL — 🛑 1 🔴 1 🟡 1phpstan-warroom-rules #33 · AC anchor: none Caution 3 finding(s) posted inline — see the review comments on the changed lines. A BLOCKER must be resolved before merge. Actionescalate — BLOCKERs present |
PR Fixer · claimed
|
…-request-to-dto # Conflicts: # CLAUDE.md # README.md # extension.neon
PR Fixer · pushed
|
…taBaseClass defaults The shipped extension.neon parameter defaults used double-backslash single-quoted FQCNs. NEON only unescapes \\ inside double-quoted strings; in single quotes the backslashes stay literal, so the value decoded to a class name matching nothing and ClassReflection::isSubclassOf() returned false for every analysed class — both EnforceFormRequestToDtoRule and (pre-existing, since PR #20) EnforceResourceDataValidatorOptInRule were silent no-ops for any consumer using the default. CI stayed green because every test constructs the rule directly via the PHP ::class constructor default, never exercising the NEON registration path. Fixes (PR #33 review — jasperboerhof BLOCKER + MAJOR + MINOR, General-review concerns): - extension.neon: single-backslash defaults + inline NEON-quoting warning. - container-resolved regression test per rule (getAdditionalConfigFiles + getByType) pinning the NEON default and %parameter% wiring; both confirmed to fail when the double-backslash defect is reintroduced. - TraitProvidedToDtoRequest fixture + test (trait-provided toDto() satisfies the contract via hasNativeMethod trait flattening). - TransitiveViolatorRequest fixture + test (concrete leaf extends abstract base without toDto() — transitive omission must fire at the leaf). Gates: composer test (107), phpstan, format:check, coverage (88.06% >= 83) green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review addressed —
|
Review Loop · 9/10 · PASSphpstan-warroom-rules #33 · AC anchor: No anchor (no issue_key / plan_dir / PR AC section) — surface anchor: package CLAUDE.md + ADR-0012/0021 + war-room queue #55 · head Tip No findings — clean against the review checklist. Actionmerge-ready |
The `## War Room ADR Projections` section was last synced 2026-05-08 and mis-described what the package distributes. Reconciled every projection bullet against the 11 rules shipping on `main`, mapping each by its docblock "Doctrine source" line per ADR-0021. - ADR-0009: rule released in v0.3.0 (was stale `Phase 2, [Unreleased]`). - ADR-0011: add `ForbidEloquentMutationInControllersRule` (PR #28). - ADR-0012: was "no HTTP surface" — now ships `EnforceFormRequestToDtoRule` (PR #33). - ADR-0019: add `ForbidEloquentMutationInControllersRule`; retire stale `EnforceExplicitHydrationRule` Phase-2 candidate note. - ADR-0029: ADDED — ships `EnforceAuditTransactionScopeRule` (PR #27). - ADR-0001: add `LogBuilderTruncateRule` to the §Append-only bullet. - New subsection for war-room Architectural-Principle rules with no published ADR: `ForbidAbortHelperRule` + `EnforceCurrentUserAttributeRule` (PR #26, Principle #9). - Bump Last synced to 2026-06-15. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Mission
Extract the FormRequest instance of war-room enforcement queue #55 (opt-in convention enforcement — "arch test detects misuse but not omission") into a canonical PHPStan rule. Sister of PR #20 (
EnforceResourceDataValidatorOptInRule, queue #55 instance 3) — same opt-in-omission pedagogy, same parameterized-base shape. War-room board item: WR-0066.Source of truth
entreezuil's reflection-based Pest arch test
tests/Arch/FormRequestsTest.php("form requests with mutation actions define toDto method") — the stronger omission-detecting form. kendo carries only the weaker misuse-only shape; the entreezuil semantic is what ships here.The rule
EnforceFormRequestToDtoRule— flags concrete classes extendingIlluminate\Foundation\Http\FormRequestthat neither declare nor inherit atoDto()method. Doctrine: ADR-0012 (FormRequest → DTO Flow). Identifier:enforceFormRequestToDto.missingToDtoMethod.BaseFormRequestintermediate is exempt by shape, not by name.toDto()satisfies the contract —method_exists()parity with the source-of-truth Pest matcher.formRequestBaseClassPHPStan parameter (defaultIlluminate\Foundation\Http\FormRequest), wired via theparameters:+parametersSchema:+arguments:triplet — the PR engineer: add EnforceResourceDataValidatorOptInRule (queue #55 Phase-2 promotion) #20 convention.LoginRequestprecedent, read-only filter requests) migrate to consumerphpstan.neonignoreErrors— never by name in rule code, per package convention. README documents the recipe.Versioning
Per ADR-0021 §Versioning: candidate Major bump — the rule surfaces new errors in already-clean code wherever a consumer has a concrete FormRequest without
toDto()(read-only/query requests included until suppressed). Pre-cascade audit required across ISMS, kendo, emmie, entreezuil, ublgenie, brick-inventory, codebook before tagging; flagged in the CHANGELOG[Unreleased]entry. Release-PR work is out of scope here.Implementation note worth reviewer attention
The constructor default uses
FormRequest::class(compile-time constant, never autoloads) instead of an FQCN string literal. Pint'sclass_keywordfixer callsclass_exists()on class-shaped string literals, and the Pint phar bundles a realIlluminate\Foundation\Http\FormRequestwhoseValidatesWhenResolvedTraitdependency is not bundled — a bare'Illuminate\Foundation\Http\FormRequest'string literal anywhere in this package's PHP source makescomposer formatfatal with "Trait not found" (minimal reproduction confirmed against an isolated temp file). Theuseimport is alias-only; consumers analysing non-Laravel trees are unaffected because::classresolution requires no autoload. Documented in the rule docblock.Gates (all local, fresh
composer install, PCOV on — CI parity)composer test— 91 tests / 142 assertions green (6 new)composer phpstan— level max clean (10 rule services registered)composer format:check— Pint cleancomposer test:coverage+composer coverage:check— 88.24% (570/646) ≥ 83% gatecomposer mutation:ci— MSI 82.94% / Covered MSI 82.94% (both ≥ 75 gate); zero escaped mutants on the new rulecomposer audit— cleanScope
Out of scope per orders: the routes-
->can()sister (queue #55 instance 1, WR-0067), the issue #14 ratchet (WR-0064), cross-territory adoption, and the release PR.🤖 Generated with Claude Code